Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Governor/custom reset release timer delay #1

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

pleasew8t
Copy link
Owner

@pleasew8t pleasew8t commented Jun 13, 2024

Description

This pull request adds an additional parameter to the governor-reset-release-timer admin command, num_days. num_days allows operators to specify the number of days a pending VAA's release needs to be reset to. This allows operators to specify delays larger than the current default (24 hours).

The parameter is optional and, if omitted, will default to the current implementation's reset value of 24 hours. If specified, the parameter may not be less than 1 day, or exceed 7 days, otherwise the release time reset will fail.

Use-case

This feature is particularly useful in scenarios where follow-up delays beyond 24 hours are anticipated, but participation by guardians cannot be guaranteed. For example, a VAA gets enqueued by the governor (imposing a delay of 24 hours), but some guardians opt to delay the VAA's release even further.

If one of these guardians recognizes that they will not be available to reset the delay over the course of the following two days, the said guardian could use the governor-reset-release-timer command to proactively delay the release to 2, or even 3, days.

Summary of code changes

  • Updated the ClientChainGovernorResetReleaseTimerCmd admin client command
  • Added a num_days field to the ChainGovernorResetReleaseTimerRequest protobuf message
  • Updated the ChainGovernorResetReleaseTimer RPC service
  • Added the TestNumDaysForReleaseTimerReset governor test

node/cmd/guardiand/adminclient.go Outdated Show resolved Hide resolved
node/cmd/guardiand/adminclient.go Outdated Show resolved Hide resolved
node/pkg/governor/governor_monitoring.go Outdated Show resolved Hide resolved
@pleasew8t pleasew8t marked this pull request as draft June 14, 2024 10:18
Copy link

@SEJeff SEJeff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor nits, but this looks pretty good from a cursory overview.

node/cmd/guardiand/adminclient.go Outdated Show resolved Hide resolved
node/cmd/guardiand/adminclient.go Outdated Show resolved Hide resolved
node/pkg/adminrpc/adminserver.go Outdated Show resolved Hide resolved
node/pkg/governor/governor_test.go Outdated Show resolved Hide resolved
node/pkg/governor/governor_test.go Outdated Show resolved Hide resolved
Copy link

@SEJeff SEJeff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you submit this to the main wormhole repo, I will ✅

@pleasew8t pleasew8t force-pushed the governor/custom-reset-release-timer-delay branch from b8d2c6e to c55afe4 Compare June 19, 2024 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants